src/main/webapp/app/shared/metis/posting-reactions-bar/posting-reactions-bar.component.scss (1)

16-16: LGTM! Consider grouping related flexbox properties.

The addition of display: flex improves the layout consistency of reaction buttons. However, for better maintainability, consider grouping related flexbox properties together.

 .reaction-button {
     background: var(--reaction-button-default-color);
     color: var(--metis-gray);
     border: none;
     border-radius: 1rem;
+    display: flex;
     align-items: center;
     height: 1.2rem;
-    display: flex;

src/main/webapp/app/shared/metis/posting.directive.ts (2)

12-12: Consider adding runtime validation for moderation rights

While the transition to the new input() function is good, consider adding runtime validation to ensure the permission flag cannot be manipulated:

-    hasChannelModerationRights = input<boolean>(false);
+    hasChannelModerationRights = input<boolean>(() => {
+        const value = false;
+        // Ensure the value is strictly boolean
+        return typeof value === 'boolean' ? value : false;
+    });

---

Line range hint 82-90: Add permission checks to edit and delete methods

Given that this PR focuses on fixing edit/delete permissions, these methods should verify the user has appropriate rights before proceeding. Currently, a malicious user could bypass UI controls and invoke these methods directly.

    editPosting() {
+        if (!this.hasChannelModerationRights()) {
+            console.warn('Unauthorized attempt to edit posting');
+            return;
+        }
        this.reactionsBar.editPosting();
        this.showDropdown = false;
    }

    deletePost() {
+        if (!this.hasChannelModerationRights()) {
+            console.warn('Unauthorized attempt to delete posting');
+            return;
+        }
        this.reactionsBar.deletePosting();
        this.showDropdown = false;
    }

src/main/webapp/app/shared/metis/posting-reactions-bar/answer-post-reactions-bar/answer-post-reactions-bar.component.ts (1)

74-76: Consider refactoring the permission check for better maintainability.

While the permission logic is correct and aligns with the PR objectives, consider extracting the complex permission check into a dedicated method for better readability and maintainability.

+  private canEditOtherUsersAnswer(): boolean {
+    const isCourseWideChannel = getAsChannelDTO(this.posting.post?.conversation)?.isCourseWide ?? false;
+    const isAtLeastInstructorInCourse = this.metisService.metisUserIsAtLeastInstructorInCourse();
+    return (isCourseWideChannel && isAtLeastInstructorInCourse) || (this.hasChannelModerationRights() ?? false);
+  }
+
   setMayEditOrDelete(): void {
     this.isAuthorOfOriginalPost = this.metisService.metisUserIsAuthorOfPosting(this.posting.post!);
     this.isAnswerOfAnnouncement = getAsChannelDTO(this.posting.post?.conversation)?.isAnnouncementChannel ?? false;
-    const isCourseWideChannel = getAsChannelDTO(this.posting.post?.conversation)?.isCourseWide ?? false;
-    const isAtLeastInstructorInCourse = this.metisService.metisUserIsAtLeastInstructorInCourse();
-    const mayEditOrDeleteOtherUsersAnswer = (isCourseWideChannel && isAtLeastInstructorInCourse) || (this.hasChannelModerationRights() ?? false);
+    const mayEditOrDeleteOtherUsersAnswer = this.canEditOtherUsersAnswer();
     this.mayEditOrDelete = !this.isReadOnlyMode && (this.isAuthorOfPosting || mayEditOrDeleteOtherUsersAnswer);
     this.mayEditOrDeleteOutput.emit(this.mayEditOrDelete);
   }

src/main/webapp/app/shared/metis/answer-post/answer-post.component.ts (2)

Line range hint 1-142: Consider implementing OnDestroy to prevent memory leaks.

The component maintains a static reference activeDropdownPost and uses document listeners, but doesn't clean them up properly.

Consider implementing the following changes:

- export class AnswerPostComponent extends PostingDirective<AnswerPost> {
+ export class AnswerPostComponent extends PostingDirective<AnswerPost> implements OnDestroy {
     // ... existing code ...
+    ngOnDestroy() {
+        // Clean up static reference if this was the active dropdown
+        if (AnswerPostComponent.activeDropdownPost === this) {
+            AnswerPostComponent.activeDropdownPost = null;
+        }
+        this.enableBodyScroll();
+    }

---

Line range hint 89-120: Consider extracting dropdown positioning logic to a service.

The dropdown positioning logic in onRightClick and adjustDropdownPosition could be reused across other components. Consider extracting it to a shared service for better code reuse as per the coding guidelines.

Example structure:

@Injectable({ providedIn: 'root' })
export class DropdownPositionService {
    adjustPosition(event: MouseEvent, dropdownWidth: number): { x: number; y: number } {
        const x = event.clientX;
        const y = event.clientY;
        const screenWidth = window.innerWidth;
        
        return {
            x: x + dropdownWidth > screenWidth ? screenWidth - dropdownWidth - 10 : x,
            y
        };
    }
}

src/main/webapp/app/shared/metis/posting-reactions-bar/post-reactions-bar/post-reactions-bar.component.html (1)

Line range hint 97-122: Consider adding error handling for permission denied scenarios.

While the permission check through mayEditOrDelete is correct, consider adding user feedback for permission denied scenarios. This would improve user experience by clearly communicating why certain actions are not available.

<button
    class="reaction-button clickable px-2 fs-small edit"
    [disabled]="readOnlyMode"
    (click)="editPosting()"
+   [ngbTooltip]="mayEditOrDelete ? ('artemisApp.metis.editPosting' | artemisTranslate) : ('artemisApp.metis.noPermission' | artemisTranslate)"
-   [ngbTooltip]="'artemisApp.metis.editPosting' | artemisTranslate"
>

src/test/javascript/spec/component/shared/metis/postings-footer/post-footer/post-footer.component.spec.ts (1)

155-162: Test implementation looks good, consider adding edge cases.

The test follows good practices with proper setup, specific assertions, and clear naming. However, consider adding test cases for:

• Error scenarios when modal closure fails
• Component state verification after modal closure
• Multiple successive open/close operations

it('should handle modal close errors gracefully', () => {
    component.posting = metisPostExerciseUser1;
    component.ngOnInit();
    fixture.detectChanges();
    const createAnswerPostModalClose = jest.spyOn(component.createAnswerPostModalComponent, 'close')
        .mockImplementation(() => { throw new Error('Modal close failed'); });
    
    expect(() => component.closeCreateAnswerPostModal()).not.toThrow();
    expect(createAnswerPostModalClose).toHaveBeenCalledOnce();
});

it('should maintain consistent state after modal operations', () => {
    component.posting = metisPostExerciseUser1;
    component.ngOnInit();
    fixture.detectChanges();
    
    component.openCreateAnswerPostModal();
    component.closeCreateAnswerPostModal();
    component.closeCreateAnswerPostModal(); // Double close
    
    expect(component.createAnswerPostModalComponent.close).toHaveBeenCalledTimes(2);
});

src/main/webapp/app/shared/metis/posting-reactions-bar/post-reactions-bar/post-reactions-bar.component.ts (2)

68-76: Consider adding error handling for event emissions

The new methods for managing answer view state are well-structured, but could benefit from error handling for edge cases.

Consider wrapping the event emissions in try-catch blocks:

 openAnswerView() {
+    try {
         this.showAnswersChange.emit(true);
         this.openPostingCreateEditModal.emit();
+    } catch (error) {
+        console.error('Failed to open answer view:', error);
+    }
 }

 closeAnswerView() {
+    try {
         this.showAnswersChange.emit(false);
         this.closePostingCreateEditModal.emit();
+    } catch (error) {
+        console.error('Failed to close answer view:', error);
+    }
 }

---

203-205: Improve readability of permission logic

The permission check is correct but could be more readable by extracting the complex condition into a well-named variable.

Consider this refactor for improved readability:

-        const mayEditOrDeleteOtherUsersAnswer = (isCourseWideChannel && this.isAtLeastInstructorInCourse) || (this.hasChannelModerationRights() ?? false);
+        const hasInstructorPermissionsInCourseWideChannel = isCourseWideChannel && this.isAtLeastInstructorInCourse;
+        const hasChannelModeratorPermissions = this.hasChannelModerationRights() ?? false;
+        const mayEditOrDeleteOtherUsersAnswer = hasInstructorPermissionsInCourseWideChannel || hasChannelModeratorPermissions;
         this.mayEditOrDelete = !this.readOnlyMode && !this.previewMode && (this.isAuthorOfPosting || mayEditOrDeleteOtherUsersAnswer);
         this.mayEditOrDeleteOutput.emit(this.mayEditOrDelete);

src/main/webapp/app/shared/metis/post/post.component.ts (2)

93-93: LGTM! Note on inheritance design.

The change from private to protected enables inheritance-based code reuse, which aligns with the component's architecture and the PR's design consistency objectives.

Consider documenting the inheritance use case in the class-level JSDoc to help future maintainers understand the design decision.

---

221-226: Add error handling and enhance documentation.

While the method implementation is clean and follows the single responsibility principle, it could benefit from defensive programming and better documentation.

Consider these improvements:

 /**
- * Close create answer modal
+ * Closes the answer post creation modal by delegating to the PostFooterComponent.
+ * This method is part of the reply functionality enhancement.
+ * @throws Error if postFooterComponent is not initialized
  */
 closeCreateAnswerPostModal() {
+    if (!this.postFooterComponent) {
+        throw new Error('PostFooterComponent is not initialized');
+    }
     this.postFooterComponent.closeCreateAnswerPostModal();
 }

src/test/javascript/spec/component/shared/metis/post/post.component.spec.ts (1)

173-180: LGTM! Consider improving test readability.

The test case correctly verifies the modal closing functionality and follows the established testing patterns. The spy assertion uses the recommended toHaveBeenCalledOnce() matcher.

Consider renaming the spy variable to better reflect its purpose:

-        const postFooterOpenCreateAnswerPostModal = jest.spyOn(component.postFooterComponent, 'closeCreateAnswerPostModal');
+        const postFooterCloseModalSpy = jest.spyOn(component.postFooterComponent, 'closeCreateAnswerPostModal');

src/test/javascript/spec/component/shared/metis/postings-reactions-bar/post-reactions-bar/post-reactions-bar.component.spec.ts (2)

220-287: Consider using test.each for permission scenarios

While the test suite is well-structured and comprehensive, consider using test.each to make the permission tests more maintainable and concise. This would reduce code duplication and make it easier to add new permission scenarios.

Example refactor:

-    it('should allow edit/delete when user is the author and not in read-only or preview mode', () => {
-        component.isAuthorOfPosting = true;
-        jest.spyOn(metisService, 'metisUserIsAtLeastInstructorInCourse').mockReturnValue(false);
-        jest.spyOn(component, 'hasChannelModerationRights').mockReturnValue(false);
-
-        component.setMayEditOrDelete();
-
-        expect(component.mayEditOrDelete).toBeTrue();
-        expect(component.mayEditOrDeleteOutput.emit).toHaveBeenCalledWith(true);
-    });
-
-    it('should allow edit/delete when user has channel moderation rights', () => {
-        // ... similar setup
-    });
+    it.each`
+        scenario                    | isAuthor  | hasModRights | readOnly | preview | expected
+        ${'author'}                 | ${true}   | ${false}     | ${false} | ${false} | ${true}
+        ${'moderator'}             | ${false}  | ${true}      | ${false} | ${false} | ${true}
+        ${'read-only'}             | ${true}   | ${true}      | ${true}  | ${false} | ${false}
+        ${'preview'}               | ${true}   | ${true}      | ${false} | ${true}  | ${false}
+        ${'no permissions'}        | ${false}  | ${false}     | ${false} | ${false} | ${false}
+    `('should $scenario scenario work correctly', 
+        ({isAuthor, hasModRights, readOnly, preview, expected}) => {
+        component.isAuthorOfPosting = isAuthor;
+        component.readOnlyMode = readOnly;
+        component.previewMode = preview;
+        jest.spyOn(component, 'hasChannelModerationRights')
+            .mockReturnValue(hasModRights);
+
+        component.setMayEditOrDelete();
+
+        expect(component.mayEditOrDelete).toBe(expected);
+        expect(component.mayEditOrDeleteOutput.emit)
+            .toHaveBeenCalledWith(expected);
+    });

---

375-393: Enhance test descriptions for modal interaction tests

While the tests correctly verify the modal interactions, consider making the test descriptions more specific to better document the expected behavior.

Example improvement:

-    it('should emit showAnswersChange and openPostingCreateEditModal when openAnswerView is called', () => {
+    it('should show answers and open create/edit modal when initiating a reply', () => {

-    it('should emit showAnswersChange and closePostingCreateEditModal when closeAnswerView is called', () => {
+    it('should hide answers and close create/edit modal when canceling a reply', () => {

Configuration used: .coderabbit.yaml
Review profile: CHILL

#!/bin/bash
# Check Angular version in package.json
jq -r '.dependencies["@angular/core"]' package.json

#!/bin/bash
# Search for other usages of hasChannelModerationRights to ensure consistent implementation
rg -t html "hasChannelModerationRights" --no-heading

# Look for the method definition in the component
ast-grep --pattern 'hasChannelModerationRights() {
  $$$
}'

#!/bin/bash
# Look for the method definition in TypeScript files
ast-grep --pattern 'hasChannelModerationRights() {
  $$$
}'

# Search for class property definitions
ast-grep --pattern 'hasChannelModerationRights: boolean'

# Search for the method implementation in component files
rg -t typescript "hasChannelModerationRights" -A 3

#!/bin/bash
# Search for TypeScript/JavaScript files containing hasChannelModerationRights
rg "hasChannelModerationRights" --type-list
rg "hasChannelModerationRights" -t js -t ts -A 3

# Find all TypeScript/JavaScript files in the component directories
fd "\.(ts|js)$" src/main/webapp/app/shared/metis/answer-post/

#!/bin/bash
# Search for other components that might be affected by scroll manipulation
rg -l "thread-answer-post" --type ts --type html

# Check for other scroll manipulation in the codebase
rg "overflow|scroll" --type ts --type html

#!/bin/bash
# Search for other components that might be using the thread-answer-post class
ast-grep --pattern 'class="$$$thread-answer-post$$$"'

# Search for other components that might be manipulating scroll behavior
ast-grep --pattern 'disableBodyScroll() {
  $$$
}'

# Search for other components that might be enabling scroll behavior
ast-grep --pattern 'enableBodyScroll() {
  $$$
}'

#!/bin/bash
# Search for thread-answer-post class usage in HTML files
rg -l 'class="[^"]*thread-answer-post[^"]*"' --type html

# Search for scroll manipulation methods in TypeScript files
rg -A 5 'disableBodyScroll|enableBodyScroll' --type ts

# Search for components that might be affected by scroll manipulation
rg -A 5 'overflow.*hidden|overflow.*auto' --type ts --type html

#!/bin/bash
# Description: Verify template updates and permission checks

# Check if all templates have been updated to use the new syntax
echo "Checking templates for old @Input syntax..."
rg -l "hasChannelModerationRights" --type html

# Check if permission checks are consistently implemented
echo "Checking permission implementations..."
rg -A 5 "hasChannelModerationRights" --type ts

#!/bin/bash
# Check the actual content of post-footer.component.html to verify template bindings
echo "Checking post-footer.component.html..."
cat src/main/webapp/app/shared/metis/posting-footer/post-footer/post-footer.component.html

# Check other templates that use hasChannelModerationRights
echo -e "\nChecking other key templates..."
cat src/main/webapp/app/shared/metis/posting-thread/posting-thread.component.html
cat src/main/webapp/app/shared/metis/answer-post/answer-post.component.html

#!/bin/bash
# Description: Verify the implementation of closeCreateAnswerPostModal in PostFooterComponent

# Search for the method implementation
echo "Searching for closeCreateAnswerPostModal implementation..."
ast-grep --pattern 'closeCreateAnswerPostModal() {
  $$$
}'

# Search for related modal component references
echo "Searching for createAnswerPostModalComponent references..."
rg "createAnswerPostModalComponent" --type ts

#!/bin/bash
# Description: Check usage of hasChannelModerationRights in templates
# Look for both property binding and method invocation patterns

echo "Checking template usage patterns..."
rg -t html "hasChannelModerationRights" --context 2

echo "Checking component references..."
rg -t typescript "hasChannelModerationRights" --context 2

#!/bin/bash
# Let's check TypeScript files specifically using ripgrep with the correct file extension pattern

echo "Checking TypeScript component references..."
rg -g "*.ts" "hasChannelModerationRights" --context 2

echo "Checking the specific component implementation..."
cat "src/main/webapp/app/shared/metis/posting-reactions-bar/post-reactions-bar/post-reactions-bar.component.ts"

#!/bin/bash
# Search for all usages of hasChannelModerationRights to ensure consistent implementation
rg -t ts "hasChannelModerationRights" -A 5

#!/bin/bash
# Search for reactionsBarComponent definition and openAnswerView method
ast-grep --pattern 'class $_ {
  $$$
  reactionsBarComponent$_
  $$$
}'

ast-grep --pattern 'openAnswerView() {
  $$$
}'

#!/bin/bash
# Description: Verify permission checks implementation
# Look for permission check usage and related components

# Check for permission-related methods and properties
echo "Searching for permission check implementations..."
rg -A 5 "mayEditOrDelete|hasChannelModerationRights|onMayEditOrDelete" --type ts

# Check for permission usage in templates
echo "Searching for permission checks in templates..."
rg -A 5 "\[mayEditOrDelete\]|\*ngIf.*mayEditOrDelete" --type html

# Look for test coverage of permission checks
echo "Searching for permission-related tests..."
rg -A 5 "describe.*permission|it.*permission|mayEditOrDelete" --type spec

src/main/webapp/app/shared/metis/post/post.component.scss (1)

23-23: Consider improving the hover menu's height handling

The fixed max-height: 2.2rem might cause issues:

1. Content could get clipped if there are multiple action items
2. It may not accommodate different font sizes or zoom levels

Consider this alternative approach:

.hover-actions {
-    max-height: 2.2rem;
+    max-height: fit-content;
+    min-height: 2.2rem;
+    overflow: visible;
}

Also, since this component contains interactive elements that appear/disappear, consider adding appropriate ARIA attributes and keyboard navigation support for better accessibility.

Configuration used: .coderabbit.yaml
Review profile: CHILL

src/main/webapp/app/shared/metis/posting-reactions-bar/post-reactions-bar/post-reactions-bar.component.html (3)

15-15: Maintain consistent button styling.

The collapse and expand buttons have inconsistent styling:

• Line 15: Uses jhi-emoji for the close icon
• Line 24: Uses fa-icon for the expand icon

Consider using the same icon system for better maintainability and visual consistency.

-                    <button class="reaction-button clickable collapse-answers-btn" [class.reaction-button--reacted]="showAnswers" (click)="closeAnswerView()">
-                        <jhi-emoji [emoji]="closeCrossId" />
+                    <button class="reaction-button clickable collapse-answers-btn" [class.reaction-button--reacted]="showAnswers" (click)="closeAnswerView()">
+                        <fa-icon class="fa-xs align-self-center" [icon]="faTimes" />

Also applies to: 23-24

---

Line range hint 48-93: Consider adding ARIA labels for better accessibility.

While the reaction buttons implementation is functionally correct, consider adding ARIA labels to improve accessibility for screen readers.

                 <button
                     [disabled]="readOnlyMode"
                     class="reaction-button clickable"
                     [class.reaction-button--reacted]="reactionMetaData.value.hasReacted"
                     (click)="updateReaction(reactionMetaData.key)"
                     [ngbTooltip]="reactionMetaData.value.reactingUsers | reactingUsersOnPosting | async"
+                    [attr.aria-label]="'React with ' + reactionMetaData.key"
+                    [attr.aria-pressed]="reactionMetaData.value.hasReacted"
                 >

---

Line range hint 93-127: Enhance keyboard navigation for emoji selector.

Consider adding keyboard navigation support for the emoji selector to improve accessibility and user experience.

                 <button
                     class="reaction-button clickable px-2 fs-small"
                     (click)="showReactionSelector = !showReactionSelector"
+                    (keydown.escape)="showReactionSelector = false"
+                    role="button"
+                    aria-haspopup="true"
+                    [attr.aria-expanded]="showReactionSelector"
                     cdkOverlayOrigin
                     #trigger="cdkOverlayOrigin"
                     [title]="'artemisApp.metis.chooseReaction' | artemisTranslate"
                 >

Configuration used: .coderabbit.yaml
Review profile: CHILL

#!/bin/bash
# Search for mayEditOrDelete usage and its implementation
rg -A 5 "mayEditOrDelete" --type ts

#!/bin/bash
# Let's search for the component file to understand the permission implementation
fd "post-reactions-bar.component.ts" --type f

# After finding the file, let's examine its content
rg -A 10 "canPin|mayEditOrDelete" --type ts

# Let's also check for any permission-related interfaces or types
rg -A 5 "interface.*Permission|type.*Permission" --type ts

#!/bin/bash
# Search for hideAfter5Seconds implementation
rg "hideAfter5Seconds" --type css

#!/bin/bash
# Search for hideAfter5Seconds implementation with context
rg -A 5 "hideAfter5Seconds" --type scss

#!/bin/bash
# Search for any JavaScript/TypeScript implementation related to hiding functionality
ast-grep --pattern 'class $_ {
  $$$
  hideAfter5Seconds($$$) {
    $$$
  }
  $$$
}'

#!/bin/bash
# Search for any timer-related code in the component
rg "setTimeout.*hideAfter" -A 5

#!/bin/bash
# Search for hideAfter5Seconds in all files
rg "hideAfter5Seconds" -A 5

# Search for CSS files with correct extension
fd -e css -e scss | xargs cat

#!/bin/bash
# Search for hideAfter5Seconds animation implementation
rg "hideAfter5Seconds|cssAnimation" -A 5 --type scss --type css

# Search for any JavaScript implementation related to hiding functionality
rg "hideAfter5Seconds" -A 5 --type ts --type js

#!/bin/bash
# Search for hideAfter5Seconds animation implementation in all files
rg "hideAfter5Seconds|cssAnimation" -A 5

# Search for any animation-related CSS/SCSS files
fd -e css -e scss | xargs grep -l "animation"

src/test/javascript/spec/component/shared/metis/postings-reactions-bar/post-reactions-bar/post-reactions-bar.component.spec.ts (1)

338-356: LGTM: Comprehensive tests for answer view interactions

The new test cases thoroughly verify the behavior of openAnswerView and closeAnswerView methods, ensuring proper event emissions. This aligns well with the PR objectives of fixing reply functionality issues.

Consider grouping these related tests under a describe block for better organization:

+  describe('Answer View Interactions', () => {
     it('should emit showAnswersChange and openPostingCreateEditModal when openAnswerView is called', () => {
       // ... existing test code ...
     });

     it('should emit showAnswersChange and closePostingCreateEditModal when closeAnswerView is called', () => {
       // ... existing test code ...
     });
+  });

Configuration used: .coderabbit.yaml
Review profile: CHILL

#!/bin/bash
# Description: Check the implementation of authority-related methods

# Look for setUserProperties implementation in parent class
ast-grep --pattern 'setUserProperties() {
  $$$
}'

# Check for any authority-related conditions
rg -A 5 'hasChannelModerationRights|isAtLeastInstructor|userAuthority' src/main/webapp/app/shared/metis/

#!/bin/bash
# Look for setUserProperties implementation in the component and its parent classes
ast-grep --pattern 'setUserProperties() {
  $$$
}'

# Check for any other method that might be setting user properties
rg -A 5 'setUserProperties|currentUser|posting\.author' src/main/webapp/app/shared/metis/posting-header/

#!/bin/bash
# Check MetisService implementation for user authority checks
rg -A 5 'metisUserIsAuthorOfPosting|metisUserIsAtLeastTutorInCourse|metisUserIsAtLeastInstructorInCourse' src/main/webapp/app/shared/metis/metis.service.ts